Attached testcase causes: (m-c-1464872875-opt-asan) ==19004==ERROR: AddressSanitizer: use-after-poison on address 0x6250006ac5a0 at pc 0x7f189d1a3527 bp 0x7ffe66ca7630 sp 0x7ffe66ca7628 READ of size 4 at 0x6250006ac5a0 thread T0 #0 0x7f189d1a3526 in TopLeft /builds/slave/m-cen-l64-asan-000000000000000/build/src/obj-firefox/dist/include/mozilla/gfx/BaseRect.h:314 #1 0x7f189d1a3526 in nsRect /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/generic/nsIFrame.h:672 #2 0x7f189d1a3526 in PresShell::MarkFramesInSubtreeApproximatelyVisible(nsIFrame*, nsRect const&, bool) /builds/slave/m-cen-l64-asan-000000000000000/build/src/layout/base/nsPresShell.cpp:6164 (m-c-1464948162-dbg-asan) [19144] ###!!! ASSERTION: StealFrame: can't find aChild: 'removed', file /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/generic/nsContainerFrame.cpp, line 1399 ==19144==ERROR: AddressSanitizer: use-after-poison on address 0x6250009ceda0 at pc 0x7f18495e9d1d bp 0x7fff8304c9d0 sp 0x7fff8304c9c8 READ of size 4 at 0x6250009ceda0 thread T0 #0 0x7f18495e9d1c in mozilla::gfx::BaseRect<int, nsRect, nsPoint, nsSize, nsMargin>::TopLeft() const /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dist/include/mozilla/gfx/BaseRect.h:314 #1 0x7f184c5d62f1 in nsIFrame::GetPosition() const /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/obj-firefox/dist/include/nsIFrame.h:672 #2 0x7f184c806f2c in PresShell::MarkFramesInSubtreeApproximatelyVisible(nsIFrame*, nsRect const&, bool) /builds/slave/m-cen-l64-asan-d-ntly-00000000/build/src/layout/base/nsPresShell.cpp:6164

Also saw this stack from the same testcase/build.

nsContainerFrame::RemoveFrame is trying to remove the blue frame. It has a next-in-flow that is an overflow container. This is a valid frame tree as far as I know, so the problem is that we pass 'true' to StealFrame here: https://hg.mozilla.org/mozilla-central/annotate/ec20b463c04f/layout/generic/nsContainerFrame.cpp#l167 'true' means we skip the overflow-container lists even if the frame bit says it's a an overflow container: https://hg.mozilla.org/mozilla-central/annotate/ec20b463c04f/layout/generic/nsContainerFrame.cpp#l1372 This is rather insane. It seems we have always done that, here's the version that introduce StealFrame and it also pass true: https://hg.mozilla.org/mozilla-central/annotate/10266/layout/generic/nsContainerFrame.cpp#l223 and even before that, when nsContainerFrame::RemoveFrame did the job itself, it never checked the overflow lists so roc@ was just preserving the existing behavior.

Here are all the StealFrame impls: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsContainerFrame.cpp?rev=5d9b7a5387ed#1353 http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?rev=5d9b7a5387ed#5956 http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsCanvasFrame.h?rev=5d9b7a5387ed#124 http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsColumnSetFrame.h?rev=5d9b7a5387ed#57 http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsInlineFrame.cpp?rev=5d9b7a5387ed#211 I suspect the reason we have this aForceNormal weirdness in the first place is that nsColumnSetFrame has its overflow containers on the normal lists for some reason.

I think we should remove the aForceNormal flag and let nsContainerFrame::StealFrame deal with it, i.e. if NS_FRAME_IS_OVERFLOW_CONTAINER then try to remove it from the overflow lists, then if not found there, try the normal lists. We can then remove the nsColumnSetFrame and nsCanvasFrame impls. I think that should be fine from a performance POV, because overflow containers are rare, and removing a child frame from its frame list is O(1) these days.

> nsColumnSetFrame has its overflow containers on the normal lists FTR, nsCanvasFrame says it does that too.

(FTR, the bug that roc@ fixed when adding the StealFrame call is bug 405271)

Regression from when? Even narrowing it down to an affected release ("47 is OK, 48 isn't") would help.

Those versions were examples. I don't crash in opt builds on 46 or 48 which I'd expect to if it's true framepoisoning.

(In reply to Daniel Veditz [:dveditz] from comment #12) > Regression from when? Even narrowing it down to an affected release ("47 is > OK, 48 isn't") would help. Sorry, was marked regression by accident.

(I'd bet this is technically a regression from Bug 1144096, from the perspective of the attached testcase at least -- since the testcase uses a CSS grid that's fragmented (and Bug 1144096 is where we added support for grid fragmentation). But we haven't enabled CSS grid by default in release builds yet, so no release builds should ever end up being affected by this bug.)

Comment on attachment 8761421 [details] [diff] [review] fix Review of attachment 8761421 [details] [diff] [review]: ----------------------------------------------------------------- r=me, just one wording nit: ::: layout/generic/nsContainerFrame.cpp @@ +1387,5 @@ > #endif > > + bool removed = MaybeStealOverflowContainerFrame(aChild); > + if (!removed) { > + // NOTE nsColumnSetFrame and nsCanvasFrame have its overflow containers on s/have its/have their/ (and then wrap "on" to the next line, to avoid going over 80 characters)

Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6bf952e58413 Crashtest.